Skip to content

feat(ios): route Simulator snapshots through AX bridge - #2279

Open
thymikee wants to merge 8 commits into
mainfrom
codex/2197-simulator-routing
Open

feat(ios): route Simulator snapshots through AX bridge#2279
thymikee wants to merge 8 commits into
mainfrom
codex/2197-simulator-routing

Conversation

@thymikee

@thymikee thymikee commented Sep 4, 2026

Copy link
Copy Markdown
Member

Summary

Closes #2197. Eligible local iOS Simulator snapshots use the packaged AX bridge and existing TypeScript presenter, with typed, generation-scoped XCTest fallback. Unknown generations remain incomparable; regular depth is presentation-only and raw depth bounds acquisition.

Target discovery is reused only while OS process-start identity matches. Native acquisition verifies the primary foreground owner before and after capture, preventing app-only trees beneath system dialogs. XCTest retains modal resolution; fallback disables the bridge until that app generation changes. Existing provider, physical-device, interaction, and prewarm routes are preserved.

Rebased onto 7a2d48d160 for the updated bundling/report schema. Native compiler setup is bounded separately from its test hook; lifecycle tests mock runner reset and synchronize crashes with real dispatch.

Scope: 34 files, 1,274 gross lines. The maintainer approved the scoped budget exception for required correctness and regression coverage.

Validation

Head: 944edfca38; base: 7a2d48d160.

  • Focused tests: 23 passed. Planted-red evidence covers target identity, native ownership, test selection, and the observed lifecycle test races.
  • Live proof on runtime-identical 2931cbc944: normal bridge capture, then foreground-owner-unverified and exactly five system-dialog nodes. Earlier proof also verifies dismissal keeps the circuit disabled and relaunch restores the bridge with a new PID.
  • Required exact-head pnpm check:affected --run is pending in the serialized local gate. Earlier full runs exposed the now-fixed test-harness failures.
  • Fresh CI and matched six-screen measurements are pending. Not merge-ready yet. Runner-demand/open latency and proxy acceptance remain perf(ios): optimize observation lifecycle and prove proxy parity #2198 scope.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.61 MB 2.64 MB +35.1 kB
JS gzip 879.1 kB 890.6 kB +11.5 kB
npm bundled raw 2.61 MB 2.64 MB +35.1 kB
npm bundled gzip 879.1 kB 890.6 kB +11.5 kB
npm tarball 1.04 MB 1.05 MB +10.8 kB
npm unpacked 3.52 MB 3.56 MB +35.9 kB
npm clean-installed 3.52 MB 3.56 MB +35.9 kB

npm unpacked components

Component Base Current Diff
JS / dist source 2.77 MB 2.80 MB +35.8 kB
Apple runner source/project 544.4 kB 544.4 kB 0 B
Apple snapshot presentation source 33.8 kB 33.8 kB 0 B
Apple Simulator snapshot bridge source 30.5 kB 30.5 kB 0 B
macOS helper source 54.8 kB 54.8 kB 0 B
Android helper artifacts 43.3 kB 43.3 kB 0 B
Other package files 46.7 kB 46.8 kB +77 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 21.9 ms 22.4 ms +0.5 ms
CLI --help 61.9 ms 64.5 ms +2.6 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/runtime4.js +5.2 kB +1.8 kB
dist/src/interaction2.js +301 B +60 B
dist/src/interactor.js +70 B +10 B

Top changed packed files

Packed file Base Current Diff
dist/src/adapter.js 0 B 27.3 kB +27.3 kB
dist/src/runtime4.js 51.3 kB 56.5 kB +5.2 kB
dist/src/ios-snapshot-planning.js 0 B 2.0 kB +2.0 kB
dist/src/runner-presentation.js 5.8 kB 5.1 kB -706 B
dist/src/sdk-selectors.d.ts 35.8 kB 36.4 kB +568 B
dist/src/ios-snapshot-runtime.js 10.2 kB 10.7 kB +488 B
dist/src/interaction2.js 33.5 kB 33.8 kB +301 B
dist/src/apps-simctl.js 0 B 264 B +264 B
dist/src/snapshot-process.js 0 B 224 B +224 B
dist/src/physical-device-control.js 14.8 kB 14.6 kB -184 B

@thymikee

thymikee commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Review at exact head 1600d40d077b02a547349396b0677d15902bb7ab: blocked on two comparison-lineage bugs plus the exact-head iOS failure.

  1. snapshot-route.ts stamps XCTest fallback with the pre-acquisition target lineage. When bridge acquisition reports a stale/relaunched target, XCTest may capture the new app while the result is labeled with the old generation. Re-resolve before fallback or make that fallback explicitly incomparable, and add a stale-generation -> fallback regression.
  2. Target-resolution failure creates a comparison identity with only stable targetId; two results with omitted generations compare equal. They can straddle an app relaunch and still be treated as one baseline. Unknown-generation fallback must fail closed across comparisons, with a closest-negative regression.
  3. Exact-head iOS Smoke is red: the regular depth-1 fixture now routes through the bridge and no longer provides the asserted snapshotQuality.backend === "tree". Reconcile the shipped output/evidence contract and rerun the iOS lane green.

The route otherwise reuses the Apple runtime/facade and shutdown seams cleanly, and the merged source lifecycle already provides authoritative OS process-start-time checks around acquisition.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Addressed all three actionable review items in fd3c6ac:

  1. Stale acquisition: stale-target failures now resolve the current target again before XCTest fallback, and the regression proves the fallback identity carries the relaunched generation.
  2. Unknown generation: resolution failures carry a typed per-capture unknown-generation residue. The closest-negative regression proves two otherwise identical unresolved fallbacks do not compare equal.
  3. iOS smoke: the depth-frontier scenario now asserts the bridge public evidence shape (no XCTest tree quality annotation plus the acquisition evidence warning) for regular and raw captures.

Also normalized fallback lineage to the declared targetId/generation fields only.

Planted-red failures were observed for items 1 and 2 before the fix. Exact-head pnpm check:affected --run is green: 731 test files, 5,589 tests. The updated iOS native lane is running on this head.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Re-review at exact head fd3c6ac226e6fe2fff5eb3aaf2ded51662aacf63 remains blocked. P1: exact-head iOS Simulator Smoke failed in the live fixture: snapshot --depth 1 published only the Application root and omitted visible-depth-projected-child. This was a successful bridge publication (not typed fallback), so it violates #2197's established regular-depth result and disproves readiness. Fix the acquisition/presentation route so bridge success preserves the visible-depth contract, then rerun the live lane. P2: the Size report shows +35.2 kB npm unpacked (+11.2 kB gzip), above the 3 kB review trigger, but the PR body does not itemize the growth or record why a materially smaller design was rejected. The architecture otherwise reuses the intended source facade, runtime binding, shared presentation engine, and interaction facade; all non-iOS checks pass.

@thymikee
thymikee force-pushed the codex/2197-simulator-routing branch from fd3c6ac to 1519222 Compare September 5, 2026 06:18
@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Follow-up from the exact-head iOS rerun: after the requested evidence assertion was corrected, the lane exposed a real bridge-depth bug rather than a test-only mismatch.

Regular --depth 1 was being sent to the native bridge as maxDepth: 1, so the TypeScript presenter never received the deeper structural nodes needed to project the visible child to presented depth 1. Fixed in 1519222: only rawTraversalDepth bounds native acquisition; regularPresentedDepth remains presentation-only.

A planted-red adapter test observed 1 instead of the bounded maximum 10, while raw depth 1 remained 1. Focused tests pass, and the rebased exact-head affected gate is green with 734 files / 5,624 tests.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Exact-head iOS rerun is green at 1519222. It passed clean-install bridge preparation, targeted XCTest regressions, public CLI preflight, Settings replay, the fixture-backed E2E scenario with the corrected depth frontier, and gesture replay. All PR checks are now green.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

The exact-head depth fix is correct: regular --depth is presentation-only again, while only rawTraversalDepth bounds native acquisition. The regression test covers the old failure, exact-head iOS Smoke includes the real projected-child assertion plus raw-depth counterexample, and all checks are green.

One readiness item remains: CI reports +35.2 kB npm unpacked (+11.2 kB gzip). Because this exceeds the repository threshold, please itemize the justified package growth and record which materially smaller designs were considered and why they were rejected. Until that rationale is recorded, I’m holding ready-for-human.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Addressed the remaining size-review item. The CI size report at 1519222 measures +35.2 kB npm unpacked and +11.2 kB JS gzip:

  • +27.2 kB: the existing adapter and its cache, lifecycle, transport, and tree-decoding dependencies become reachable from the shipped CLI. This is reused implementation, not another bridge. The facade still dynamically imports it only on the first eligible capture.
  • +4.8 kB: Apple runtime chunk growth, including routing, target resolution, bounded fallback, and generation-scoped circuit state.
  • +2.0 kB: the shared snapshot-planning chunk becomes separately emitted/reachable. Presentation still has one TypeScript owner.
  • Remaining roughly +1.2 kB net: declaration growth, other changed/shared chunks, and packaging output. This includes the -706 B runner-presentation reduction; rounded chunk deltas are not a source-line attribution.
  • Native runner, presentation, and simulator bridge source payloads are unchanged (0 B each).

Smaller alternatives considered: retaining XCTest-only routing would omit #2197; deleting XCTest would break required fallback and excluded targets; replacing the source facade with direct helper calls would duplicate or drop its existing deadline, process-generation, cache, and teardown safeguards. Moving the adapter into an optional package would move the installation cost rather than remove it, introduce a separate install/version boundary, and change the packaged-default contract. None is a justified reduction for this slice.

The repository-required independent read-only size/design review found no actionable simplification: one runtime-owned lazy source is reused and closed, with the existing presenter and fallback retained. No production-code changes were needed for this comment.

Rebased onto 0da105e; git range-diff confirms all three patches are unchanged. The new head is 37e3ef5. Prior green CI/device evidence is stamped to the old head; fresh validation is reported in the PR description.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Reviewed 37e3ef5: no remaining actionable findings. Stale/unknown lineage and regular-depth regressions are resolved; existing lazy bridge, shared presenter and typed fallback retained. The +35.2 kB growth is itemized and smaller designs addressed. Prior live depth/raw-depth evidence covers the unchanged rebased patches. Code-review clean; fresh iOS CI was pending at review, so not an unconditional merge-ready claim.

@thymikee thymikee added ready-for-human Valid work that needs human implementation, judgment, or maintainer merge and removed ready-for-human Valid work that needs human implementation, judgment, or maintainer merge labels Sep 5, 2026
@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Delta review at e84bdc1, independently reviewed: no concrete source-level findings. Device/app-scoped target reuse requires matching process start identity; the bridge checks the pinned identity before capture and current identity after it, and stale acquisition explicitly refreshes discovery before fallback. Tests cover PID reuse, unavailable identity, replacement and refresh. Existing source/presenter/lifecycle owners are reused. Readiness remains held for the promised exact-base versus head six-screen benchmark and public-response evidence, still marked in progress; prior green evidence does not cover this new warm-path delta. Exact-head iOS is also pending. Removed the label that this sentinel applied to the previous head; restore only after the required evidence is complete and no failing checks/conflicts remain.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Sentinel evidence update at unchanged e84bdc1: exact-head iOS Smoke is now failing in smoke:automation-input, waiting for Agent Device Tester. Typed reason wait_capture_stalled, readableCaptures=0, 10,000 ms budget / 10,233 ms elapsed (run 33985544618). This does not yet isolate a source defect, but requires investigation or successful exact-head validation. The promised matched six-screen benchmark/public-response evidence is also still marked in progress. Previous code review stands; readiness remains held.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Acceptance update at 7cf6e7fd3841f0e63ae56d5a9cc1459920193b41:

  • Scope exception approved: the maintainer approved keeping the required correctness fixes and regression coverage together at 1,267 gross changed lines. This is a scoped exception to the 1,000-line PR budget, not a readiness waiver.
  • Stalled iOS capture investigated: run 33985544618, at e84bdc15d4, shows the fixture home screen correctly visible. Request 712353abbe3417aa spent 4,650 ms connecting for runner findText, then target discovery hit its 3,000 ms xcrun timeout. Typed target-resolution fallback started with about 1.8 seconds left and was cancelled at the original request deadline. No bridge acquisition occurred. This identifies where the budget was spent; it does not establish the underlying cause of the slow connections or dismiss the failure as flaky. Fresh exact-head iOS validation remains required.
  • Foreground correctness: the worker observed a normal bridge capture, then opened a microphone permission dialog in the same app process. foreground-owner-unverified triggered XCTest fallback, which returned the dialog and buttons rather than the covered app tree. The native guard and automatic iOS regression are committed. Local affected gates passed: 743 files / 5,671 tests.
  • Benchmark evidence remains pending: the matched six-screen base/head corpus is not complete. A rebase is underway because upstream perf: bundle runtime dependencies and report full install size #2310 changed packaging and the size-report schema; the size job mixed reporter versions and failed before comparison. Final measurements will use the updated exact base/head, with earlier evidence retaining its original SHA attribution.

Still required before merge-readiness: publish the complete matched benchmark/public-response artifacts and obtain green final-head CI, including iOS. No new performance claim yet.

@thymikee

thymikee commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Sentinel review at 7cf6e7f: Two confirmed readiness gaps: (1) the new native-runtime.test.ts beforeAll hits the 10s hook timeout in iOS run33987768263, so its native regression never executes; use an explicit bounded compile/hook budget or smaller compilation scope and establish green CI. (2) foreground-owner.m calls only isPrimaryForegroundProcess, not snapshotForProcess: removing either new pre/post ownership guard leaves this test green. Add regression proof that deleting either guard fails and covered/changing ownership yields typed failure without publishing the app tree. The foreground-owner implementation otherwise matches the intended fallback policy. Matched benchmark/public-response artifacts remain pending, and Size has the cross-version reporter-schema failure. No readiness label.

@thymikee
thymikee force-pushed the codex/2197-simulator-routing branch from 7cf6e7f to 944edfc Compare September 5, 2026 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(ios): route Simulator snapshots through the bridge with bounded fallback

1 participant